Skip to content

fix(auth): honor disabled guest access - #162

Merged
PIKACHUIM merged 5 commits into
mainfrom
fix/issue-133-guest-access
Nov 9, 2026
Merged

PIKACHUIM merged 5 commits into
mainfrom
fix/issue-133-guest-access

Conversation

@PIKACHUIM

@PIKACHUIM PIKACHUIM commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Summary / 摘要

新装默认要求登录:初始化的 guest 账号为禁用状态。同时移除 TS 独有的 allow_guest 开关——Go 后端没有这个设置,匿名访问是否放行只由 guest 账号是否启用决定。

Fresh installs now require sign-in: the seeded guest account is created disabled. The TS-only allow_guest switch is removed — the Go backend has no such setting and decides anonymous access solely by the guest account state.

Problem / 问题

issue #133 报告的是:默认部署未登录即可浏览挂载内容,且界面/文档中找不到"要求登录"的开关。

原因不是某个鉴权判断写漏,而是默认值 + 缺少开关:

  • 默认库直接创建一个 disabled: false 的 guest,无 Authorization 的请求会回退成 guest,前端 /api/me 拿到 200 就直接渲染首页。
  • 早前实现里的 allow_guest 只是 /api/public/settings 的派生字段:Go 仓库零处匹配该键,TS 的默认设置里也没有对应行,前端不读取它,管理界面无入口。正常部署无法让它变成 false;即便为 false,也只影响"guest 已启用"这一非默认状态。

Changes / 改动

  • src/backend/internal/model/db.ts: defaultDb.users 里的 guest 改为 disabled: true;删除 allow_guest 设置行。
  • src/backend/server/auth.ts: getOrInitUsers() 占位/自愈分支创建的 guest 同样禁用(与默认库一致)。
  • src/backend/server/middlewares.ts: getUserFromContext 恢复为 Go 语义——匿名是否回退 guest 只看 guest 是否存在/被禁用,注释指向 server/middlewares/auth.go 的 Auth(false)。
  • src/backend/server/emby.ts: guestUserOf 同上(Emby 匿名封面入口只看 guest 状态)。
  • src/backend/server/public.ts: /api/public/settings 不再返回 allow_guest(Go 公开设置里没有该键);init/setup 不再写入该设置项。
  • src/backend/pkg/permission.ts: 删除 isGuestAllowed()。
  • README.md / readmes/README_en.md: 说明新装默认要求登录;需要匿名只读时在用户管理中启用 guest(与 Go 同一套语义)。
  • 测试:新增 src/backend/server/guest_defaults.test.ts;guest_auth.test.ts / public_settings.test.ts / emby_protocol.test.ts 相应调整。

行为对照(Go ↔ TS)

场景 Go 本 PR 之后的 TS
新装匿名浏览 guest 初始启用 → 可浏览 guest 初始禁用 → 401 / 跳登录页
管理员启用 guest 匿名恢复可读 同上(无第二个开关)
guest 被禁用 /api/* 401;/api/fs 非分享路径 401,分享 /@s 仍可用 一致(isShare 分流在用户检查之前)
/d /p 直链 无 Auth,仅 needSign(sign_all / 存储签名 / meta 密码) 一致(raw.ts 对齐注释)
/api/public/*、/api/public/media/thumb 显式免鉴权 一致

Breaking changes / 破坏性变更

  • /api/public/settings 不再返回 allow_guest。前端未读取该字段;第三方客户端若读取需要适配。
  • 存量部署不受影响:用户表来自持久化库,升级不会改动 guest 状态,也不会被静默切换成"必须登录"。只有新装默认改为要求登录。
  • 若某个库中存在此前手工/测试写入的 allow_guest 设置行,现在会被忽略,可在「站点设置」里删除。

Verification / 验证

npm run test:server   # 224/224 passed
npm run lint          # tsc -p tsconfig.json --noEmit 通过

新增用例覆盖:

  1. defaultDb:guest 默认禁用,且不存在 allow_guest 设置行;
  2. 新装(ADMIN_PASS 首启):清模块缓存后重读仍为禁用 + allow_guest=false 已移除,匿名 /api/fs/list 401;
  3. 新装(安装向导 init/setup):完成后 guest 仍禁用,匿名 401;
  4. 存量库(guest 启用):升级后仍可匿名访问,不被强制登录;
  5. /api/public/settings 不含 allow_guest。

Emby 现有封面用例(访客根目录 / hidden / meta / 媒体类型开关)全部保持通过。

Notes for reviewers

  • 该分支早期的两次提交(c56b8e6、23ea82a)引入了 allow_guest 执行点和新装写入,最新提交 90bdd8a 已将其整体移除;请以 Files changed 的净 diff 为准。
  • 与 Go 的对齐依据:server/router.go(/api/fs 用 Auth(true)、/d /p 无 Auth)、server/middlewares/auth.go(Auth(false) + guest.Disabled)、server/handles/fsread.go(/@s 先分流再查禁用)、server/handles/media_image.go(公开缩略图)。

Related Issue / 关联

AI Disclosure / AI 使用声明

  • 本 PR 包含 AI 辅助内容(CodeBuddy),已由提交者逐行复核并在本地完成测试复现。
  • 已验证 git diff --check、npm run test:server、npm run lint 全部通过。

Issue #133 reports that unauthenticated visitors can browse mounted content despite expecting sign-in. The public settings endpoint already exposes allow_guest=false, but getUserFromContext previously fell back to any active guest account without enforcing that setting.

When no Authorization header or query token is present, return null before guest fallback if allow_guest is false. Preserve the current default guest behavior when the setting is absent. Add a regression test proving default fallback remains enabled and that disabling it makes anonymous /api/fs/list return 401. Document that guest access is an OpenList application setting, not a Cloudflare environment variable.

Fixes #133

Validation: git diff --cached --check passed. Targeted runtime test is pending dependency installation in an isolated verification copy.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
openlist-tsworkers 90bdd8a Nov 09 2026, 10:07 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
openlist-work 90bdd8a Nov 09 2026, 10:08 AM

@PIKACHUIM PIKACHUIM linked an issue Nov 1, 2026 that may be closed by this pull request
4 tasks done
@PIKACHUIM PIKACHUIM linked an issue Nov 1, 2026 that may be closed by this pull request
4 tasks done
cluntop pushed a commit to cluntop/OpenList-Worker that referenced this pull request Nov 1, 2026
…placeholders (OpenListTeam#113)

* fix(static): repair /manifest.json 401 on cookie-gated deployments (EdgeOne preview)

Three stacked issues made /manifest.json the only failing request:

1. Frontend OpenListTeam#162 (2025-08-08) repointed index.html from /static/manifest.json
   to /manifest.json to match the Go backend dynamic route
   (OpenList server/router.go:36), but the dist still only ships
   static/manifest.json - no root manifest.json exists.
2. The TS worker never aligned that route, so /manifest.json fell through to
   the SPA fallback (text/html on Cloudflare).
3. On cookie-gated platforms (EdgeOne Makers preview domains, eo_time), every
   other request carries the auth cookie while the PWA manifest fetch is made
   without any credentials by Chromium - so manifest.json alone got 401.

Fix:
- fetch-frontend.mjs: copy static/manifest.json -> manifest.json at build
  time (covers static-first platforms like EdgeOne Pages) and add
  crossorigin="use-credentials" to the manifest <link> so the browser sends
  cookies; both idempotent, applied to every dist source path.
- assets.ts: new ensureManifestCredentials() applied to all HTML served by
  getIndexHtmlWithCdn (covers CDN-sourced index.html, path B), plus a
  /manifest.json -> /static/manifest.json 302 route aligned with the Go
  router for worker-served traffic.
- assets.test.ts: 5 new cases for the attribute patch and the CDN path.

Verified: server suite 184/184, tsc --noEmit clean, fetch-frontend end-to-end
produces dist/manifest.json and a patched index.html.

* fix(edgeone): restore server-handler detection via committed handler placeholders

EdgeOne Pages decides whether a project has a Node backend by scanning the
repository content, not the post-build filesystem: with the handler artifact
gitignored (PR OpenListTeam#93), detection fails ("No server-handler detected, generating
routes.json for pure project") and /api/* falls into the static rewrites
returning index.html - while functions/kv-* keep working.

- scripts/build-edge.mjs: emit the bundled handler to the officially
  documented node-functions/[[default]].js in addition to the historical
  cloud-functions/[[default]].js; the deploy-time build overwrites both with
  the fresh ~2MB bundle, so runtime always executes the latest code.
- cloud-functions/[[default]].js + node-functions/[[default]].js: track a
  tiny, never-changing placeholder (valid onRequest export returning 503).
  It exists only so platform detection sees a server handler; the deploy
  build overwrites it with the real bundle on the build filesystem. Merge
  conflicts on artifacts are structurally impossible and contributors never
  need to touch these files. This supersedes both PR OpenListTeam#93's
  gitignored-artifact approach and an interim full-artifact-committed one.
- middleware.js: export config ({ matcher: [{ source: "/:path*" }] }) as the
  platform expects, silencing "Could not find config in middleware file,
  using defaults".
- .github/workflows/build-verify.yml: assert the tracked entries are still
  the placeholder so accidentally committing a full build fails CI with a
  clear message.
- .gitignore: stop ignoring the two handler entry files.

Verified on a live EdgeOne deployment: /api/public/settings returns JSON
again after deploying this change.

@pikachuren pikachuren left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 感谢贡献 PR #162!感谢 @PIKACHUIM 提交~
🤖 AI 自动审核声明:本评审报告由 AI 自动生成,当前使用 DeepSeek-V4.1-Flash 模型进行分析。
⚠️ AI 分析结果仅供参考,可能存在误判或遗漏。如您发现任何问题或有不同意见,欢迎随时提出讨论和纠正。
⚠️ 重要提醒:即使 AI 评审认为代码质量良好且建议合并,最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出是否合并的决策。

🎯 结论

✅ Approve — 安全设置必须在后端执行点落实,这个修复很到位,可以合并

📖 概要

fix(auth): honor disabled guest access · 修复 allow_guest=false 只影响 /api/public/settings 展示、后端仍把匿名请求映射成 guest 用户
核心改动:getUserFromContext 在应用 guest 回退前检查 allow_guest;补充中英文 README 说明与回归用例

🧭 整体方案

把「设置项」从「前端提示」升级为「鉴权路径上的执行点」,并保持未设置/未启用时的旧行为不变。判定位置选在唯一的 guest 回退入口,改动面最小。

📊 变更统计

4 个文件(+76 / -2) | 功能 ⭐⭐⭐⭐⭐ | 最小改动 ⭐⭐⭐⭐⭐ | 前向兼容 ⭐⭐⭐⭐⭐ | 方案设计 ⭐⭐⭐⭐⭐

🚨 关键问题

无重大问题。

P2(可选):

  • 💡 server/middlewares.ts:502-508 用 allowGuest === "false" 严格比较,与 server/public.ts:514 的既有写法一致 👍 但 OpenList 的设置项是字符串存储,历史数据里若出现布尔 false 或 "False",这里会放行(fail-open)。建议抽一个 isGuestAllowed(settings) 的公共 helper(顺带把 public.ts 那处也纳进来),统一做 String(v).trim().toLowerCase() === "false" 判定。
  • 💡 现在 allow_guest 的判定分散在 public.ts(展示)与 middlewares.ts(执行),两处规则若将来只改一处,就会出现「设置页显示已关闭、实际仍可匿名访问」的复发场景。上面那个 helper 能顺带解决。

📂 逐文件分析

src/backend/server/middlewares.ts

改动意图:allow_guest=false 时不再返回 guest 用户。
代码逻辑:拿到 db 后先读 settings.allow_guest,为 "false" 直接 return null;否则走原 guest 查找(仍要求 guest 未被删除/禁用)。
评价:位置正确——该分支是 getUserFromContext 里唯一的 guest 回退点,且后面没有「无用户时兜底 admin」的引导逻辑会被跳过(我确认过 550 行之后直接进入 token 解析分支)。try/catch 与原有失败语义一致(异常时返回 null)。

src/backend/server/guest_auth.test.ts

评价:三段式断言很好——先验证默认(allow_guest 未设置时仍是 guest),再验证 "false" 时 getUserFromContext 为 null,最后用真实 /api/fs/list 请求验证 401 + body.code 401。既覆盖了「不回退」,也覆盖了「HTTP 层真的拒绝」。

README.md / readmes/README_en.md

评价:明确「这是应用设置,不是 Cloudflare 环境变量」,正好堵住 issue 里最常见的误解来源 👍

✅ 待处理清单

  • [P2] 抽 isGuestAllowed() 公共 helper,public.ts 与 middlewares.ts 共用
  • [P2] helper 内兼容布尔 false / 大小写等历史写法(避免 fail-open)

🎯 结论:✅ Approve — 修复点在正确的执行层,测试覆盖到 HTTP 层,向后兼容良好。

pikachuren and others added 4 commits November 9, 2026 17:40
…ccess

# Conflicts:
#	README.md
#	readmes/README_en.md
- Add pkg/permission.isGuestAllowed(): tolerates boolean false / "False" /
  padded values, and is shared by /api/public/settings and every auth path so
  display and enforcement cannot drift apart
- getUserFromContext uses isGuestAllowed (bare === "false" failed open)
- Emby anonymous cover entry (guestUserOf) now honors allow_guest too
- Regression tests: boolean/case/padded settings values and Emby anonymous
  cover returning 401; both verified by mutation

Co-Authored-By: CodeBuddy <noreply@codebuddy.ai>
- defaultDb: disable the guest account by default (issue #133)
- defaultDb: make allow_guest a real site setting (value "true" on insert so
  existing deployments are not silently switched to "must sign in")
- init/setup and ADMIN_PASS first boot write allow_guest=false for new installs
- docs: two-step opt-in for anonymous read-only browsing
- tests: fresh install (both init paths) is blocked and persisted; an existing
  deployment without the setting keeps its previous behavior

Co-Authored-By: CodeBuddy <noreply@codebuddy.ai>
Go has no allow_guest setting: anonymous access is decided only by whether the
guest account exists and is disabled (server/middlewares/auth.go Auth(false)).
The TS-only switch added earlier was therefore a divergence, not a fix.

- remove the default setting row and the init-time writes
- remove isGuestAllowed and the checks in getUserFromContext / Emby guestUserOf
- stop echoing allow_guest from /api/public/settings (the frontend never read it)
- keep the actual #133 fix: fresh installs create a disabled guest, so anonymous
  browsing requires an explicit opt-in by enabling the guest user
- tests assert parity: no allow_guest in defaults or public settings, fresh
  install returns 401, an existing deployment is left untouched

BREAKING CHANGE: /api/public/settings no longer returns allow_guest.

Co-Authored-By: CodeBuddy <noreply@codebuddy.ai>
@PIKACHUIM
PIKACHUIM merged commit 705f603 into main Nov 9, 2026
3 checks passed
newzi23 added a commit to newzi23/OpenList-Worker-newzi that referenced this pull request Nov 11, 2026
- 合并 upstream/main(139 驱动 token 修复、Wrangler bundler safer-buffer、WebDAV 播放 401、OpenListTeam#162 权限修复等)
- observability.enabled:免费版含 20 万事件/日、保留 3 天,此前线上为 None
- placement.mode=smart:Worker 实测在 LAX 执行,而网盘 origin 在国内
- 注:上游已移除 CACHE_* 环境变量(代码不再读取),故原计划的 CACHE_TTL 调整取消
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] 未登录用户可以看到需要登录用户的文件

2 participants